Skip to content

Conversation

@jansvoboda11
Copy link
Contributor

External commands wishing to implement their own isResultValid() check are not given enough resources to check the output files and other things ExternalCommand::isResultValid() does by default. This makes it impossible to just add some extra checks on top of the default implementation.

This PR adds new C API and extends the ProducesCustomBuildValue protocol that allow clients to fall back to the default implementation.

@jansvoboda11 jansvoboda11 requested a review from owenv July 18, 2024 20:19
@owenv
Copy link
Contributor

owenv commented Jul 18, 2024

This looks good to me. Can you bump LLBUILD_C_API_VERSION here too?

@dmbryson
Copy link
Contributor

Agreed, looks good if we bump that version.

@jansvoboda11 jansvoboda11 marked this pull request as ready for review July 23, 2024 16:27
@jansvoboda11 jansvoboda11 requested a review from dmbryson as a code owner July 23, 2024 16:27
@jansvoboda11
Copy link
Contributor Author

@swift-ci please test

@jansvoboda11
Copy link
Contributor Author

@swift-ci please smoke test

@jansvoboda11
Copy link
Contributor Author

@swift-ci please smoke test

@jansvoboda11
Copy link
Contributor Author

@swift-ci please test

@jansvoboda11 jansvoboda11 merged commit 437dac1 into main Jul 24, 2024
@jansvoboda11 jansvoboda11 deleted the jan_svoboda/better-isResultValid-API branch July 24, 2024 15:52
jansvoboda11 added a commit that referenced this pull request Jul 31, 2024
This reverts commit a3ef0f7 (#928) and essentially reapplies 437dac1 (#926). The downstream failure was not related to this change.

Note that compared to the original commit, this one also initializes the newly introduced function pointer to nil in BuildSystemBindings.swift.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants